Skip to content

[pigeon] Tidy imports and "ignore" comments#11149

Merged
auto-submit[bot] merged 12 commits intoflutter:mainfrom
srawlins:imports
Mar 11, 2026
Merged

[pigeon] Tidy imports and "ignore" comments#11149
auto-submit[bot] merged 12 commits intoflutter:mainfrom
srawlins:imports

Conversation

@srawlins
Copy link
Contributor

@srawlins srawlins commented Mar 1, 2026

  • Uint8List appears to be no longer needed.
  • The only reason foundation.dart was imported was for the annotations exported by the meta package.
  • Instead of ignoring individual lint rules, all lint rules should be ignored, so that the generated code doesn't fail users' CI, when the code is out of their control.

Pre-Review Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] page, which explains my responsibilities.
  • I read and followed the [relevant style guides] and ran [the auto-formatter].
  • I signed the [CLA].
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I [linked to at least one issue that this PR fixes] in the description above.
  • I followed [the version and CHANGELOG instructions], using [semantic versioning] and the [repository CHANGELOG style], or I have commented below to indicate which documented exception this PR falls under[^1].
  • I updated/added any relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or I have commented below to indicate which [test exemption] this PR falls under[^1].
  • All existing and new tests are passing.

@srawlins srawlins requested a review from tarrinneal as a code owner March 1, 2026 18:21
gemini-code-assist[bot]
gemini-code-assist bot reviewed Mar 1, 2026
View reviewed changes
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the Dart code generator for Pigeon. The changes include:

  • Modifying the generated ignore_for_file comments to use type=lint, which disables all lint rules for the generated files.
  • Removing the import of Uint8List from dart:typed_data in generated files.
  • Changing the source of annotations (e.g., @immutable) from the Flutter foundation library to the meta package.
    These generator changes are reflected in the updated example and test files. The package version in pubspec.yaml, CHANGELOG.md, and generator_tools.dart has been updated to 26.1.10.

@stuartmorgan-g
Copy link
Collaborator

stuartmorgan-g commented Mar 2, 2026 •
edited
Loading

  • Instead of ignoring individual lint rules, all lint rules should be ignored, so that the generated code doesn't fail users' CI, when the code is out of their control.

We should make this something we can override internally for our own test generation; while we don't want to fail client analysis, we also don't want to introduce actual problems that lints would catch without our CI telling us. (E.g., if we leave out an await where we should have one, we absolutely want our CI to fail.)

@srawlins
Copy link
Contributor Author

srawlins commented Mar 2, 2026

That's a great idea! Is there one (or a few) cental spot where I could put a flag, for code generation "for/during tests"? Are there already tests that assert that new code doesn't have warnings or lints? Or is that mainly the checked-in code-generated files?

@stuartmorgan-g
Copy link
Collaborator

stuartmorgan-g commented Mar 5, 2026

@tarrinneal can advise on the best place to put an option for this. As for tests, anything checked in is run through analyze in CI, and we also run analysis on code generated for package-level Dart unit tests, so that's where we would expect to catch issues. We could always add a new custom test step if needed though.

@tarrinneal
Copy link
Contributor

tarrinneal commented Mar 6, 2026

That's a great idea! Is there one (or a few) cental spot where I could put a flag, for code generation "for/during tests"? Are there already tests that assert that new code doesn't have warnings or lints? Or is that mainly the checked-in code-generated files?

I think just a simple bool for internal tests in the external PigeonOptions is fine, with a hidden command line param as well.

the pigeon_lib file has most of this code. The internal options class will need also need this added so it get's plumbed to all the generators.

@srawlins
Copy link
Contributor Author

srawlins commented Mar 6, 2026

Done. Thanks for the pointers!

@tarrinneal
Copy link
Contributor

tarrinneal commented Mar 10, 2026

https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#readme-code

@srawlins
Copy link
Contributor Author

srawlins commented Mar 10, 2026

Thank you, done.

tarrinneal
tarrinneal reviewed Mar 10, 2026
View reviewed changes
tarrinneal
tarrinneal approved these changes Mar 10, 2026
View reviewed changes
Copy link
Contributor

@tarrinneal tarrinneal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again! Just a quick change and it's good to go!

@tarrinneal tarrinneal added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 11, 2026
@auto-submit auto-submit bot merged commit 44980b6 into flutter:main Mar 11, 2026
81 checks passed
@srawlins srawlins deleted the imports branch March 11, 2026 02:33
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 11, 2026
@engine-flutter-autoroll
44980b664 [pigeon] Tidy imports and "ignore" comments (flutter/packag…
824d3d3 
…es#11149)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 11, 2026
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 11, 2026
@engine-flutter-autoroll
44980b664 [pigeon] Tidy imports and "ignore" comments (flutter/packag…
45dd0fb 
…es#11149)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 11, 2026
Merged
@srawlins srawlins mentioned this pull request Mar 11, 2026
Open
11 tasks
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Mar 11, 2026
@engine-flutter-autoroll
Roll Packages from ee460d6a01fe to ecace66e92c2 (10 revisions) (#183517)
dd4e378 
flutter/packages@ee460d6...ecace66

2026-03-11 srawlins@google.com [pigeon] Produce a helpful error for bad
method return type (flutter/packages#11204)
2026-03-11 srawlins@google.com [pigeon] Use hasLength and isEmpty in
tests for better failure messages (flutter/packages#11205)
2026-03-11 dacoharkes@google.com [rfw] Opt out of icon tree shaking
(flutter/packages#11216)
2026-03-11 srawlins@google.com [pigeon] Tidy imports and "ignore"
comments (flutter/packages#11149)
2026-03-11 49699333+dependabot[bot]@users.noreply.github.com
[dependabot]: Bump org.jetbrains.kotlin:kotlin-bom from 2.2.21 to 2.3.10
in /packages/pigeon/platform_tests/test_plugin/android
(flutter/packages#10984)
2026-03-11 srawlins@google.com [pigeon] Improve casting and
nullability-handling in generated code (flutter/packages#11163)
2026-03-10 engine-flutter-autoroll@skia.org Roll Flutter from
2ec61af to 195ae7b (36 revisions) (flutter/packages#11222)
2026-03-10 git@reb0.org [vector_graphics] Respect BoxFit parameter with
viewbox (flutter/packages#11012)
2026-03-10 stuartmorgan@google.com Add AI contribution guidelines to PR
checklist (flutter/packages#11195)
2026-03-10 zezohassam@gmail.com [video_player] Optimize caption
retrieval with binary search in VideoPlayerController
(flutter/packages#8347)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC flutter-ecosystem@google.com on the revert to ensure that a
human
is aware of the problem.

To file a bug in Flutter:
https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tarrinneal tarrinneal tarrinneal approved these changes

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App p: pigeon

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@srawlins @stuartmorgan-g @tarrinneal